fix(spanner): fix grpc-gcp affinity cleanup and multiplexed channel usage leaks by rahul2393 · Pull Request #12726 · googleapis/google-cloud-java

rahul2393 · 2026-04-09T07:47:48Z

Summary

This change fixes two regressions in the Spanner Java client introduced around multiplexed session initialization and
grpc-gcp affinity handling.

1. Fix static `CHANNEL_USAGE` leak in `MultiplexedSessionDatabaseClient`

Fixes: #12693
MultiplexedSessionDatabaseClient stored per-SpannerImpl channel usage state in a static map but never removed
entries on close. After multiplexed client creation became unconditional in SpannerImpl.getDatabaseClient(),
applications that repeatedly created and closed Spanner instances could retain closed SpannerImpl objects, gRPC
channels, and related transport state indefinitely.

This change:

replaces the static Map<SpannerImpl, BitSet> with reference-counted shared state
removes the map entry when the last MultiplexedSessionDatabaseClient for a given SpannerImpl closes
preserves sharing semantics for multiple database clients created from the same SpannerImpl

2. Stop using bitset / bounded `% numChannels` affinity for grpc-gcp

For grpc-gcp-enabled paths, channel affinity should use the raw random channel hint and rely on explicit unbind /
cleanup, rather than:

bitset reservation
collapsing the hint with % numChannels

This change:

keeps grpc-gcp on raw random affinity keys in GapicSpannerRpc
removes % numChannels mapping from the grpc-gcp call path
keeps the old non-grpc-gcp GAX affinity behavior unchanged

3. Add explicit grpc-gcp affinity cleanup for multi-use read-only transaction close

Multi-use read-only transactions reuse a single random channel hint for the lifetime of the transaction. That hint
should remain stable across all reads in the transaction, then be explicitly cleaned up when the transaction closes.

This change:

adds a new RPC cleanup hook that carries both transaction id and channel hint
invokes that cleanup from multi-use read-only transaction close()
unbinds grpc-gcp affinity on transaction close without issuing an ExecuteSql RPC to Spanner
handles both:
- location API disabled: cleanup via the underlying grpc-gcp channel
- location API enabled: cleanup via KeyAwareChannel, using the routed endpoint associated with the transaction

4. Apply grpc-gcp affinity cleanup settings without implicitly enabling DCP

grpc-gcp affinity cleanup settings (affinityKeyLifetime, cleanupInterval) should be applied whether or not dynamic channel pool (DCP) is enabled.

This change:

always passes grpc-gcp channel-pool options when grpc-gcp is enabled
preserves full pool settings only when DCP is enabled
passes cleanup-only channel-pool options when DCP is disabled:
- affinityKeyLifetime
- cleanupInterval
- dynamic scaling explicitly disabled

…sage leaks

gemini-code-assist

Code Review

This pull request implements explicit cleanup for gRPC-GCP channel affinity and introduces reference counting for shared channel usage in MultiplexedSessionDatabaseClient to ensure proper resource management. Key changes include the addition of clearTransactionAndChannelAffinity to the SpannerRpc interface and its implementations, allowing the system to unbind affinity keys when transactions or single-use operations complete. Additionally, GapicSpannerRpc was updated to propagate affinity cleanup settings even when dynamic channel pooling is disabled. Review feedback suggests adding a null check in getGrpcGcpChannelPoolOptions to prevent a potential NullPointerException and recommends consolidating the clearChannelHintAffinity implementation to avoid code duplication between KeyAwareChannel and GapicSpannerRpc.

gemini-code-assist · 2026-04-09T07:50:40Z

...nner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java

+    GcpChannelPoolOptions channelPoolOptions = options.getGcpChannelPoolOptions();
+    if (options.isDynamicChannelPoolEnabled()) {
+      return channelPoolOptions;
+    }
+
+    return GcpChannelPoolOptions.newBuilder()
+        .disableDynamicScaling()
+        .setAffinityKeyLifetime(channelPoolOptions.getAffinityKeyLifetime())
+        .setCleanupInterval(channelPoolOptions.getCleanupInterval())
+        .build();


The method getGrpcGcpChannelPoolOptions assumes that options.getGcpChannelPoolOptions() returns a non-null value. If it returns null, a NullPointerException will occur when calling getAffinityKeyLifetime() or getCleanupInterval(). While SpannerOptions typically provides a default, adding a null check or ensuring a non-null value is safer for robustness.

static GcpChannelPoolOptions getGrpcGcpChannelPoolOptions(SpannerOptions options) { GcpChannelPoolOptions channelPoolOptions = options.getGcpChannelPoolOptions(); if (options.isDynamicChannelPoolEnabled()) { return channelPoolOptions; } GcpChannelPoolOptions.Builder builder = GcpChannelPoolOptions.newBuilder().disableDynamicScaling(); if (channelPoolOptions != null) { builder.setAffinityKeyLifetime(channelPoolOptions.getAffinityKeyLifetime()) .setCleanupInterval(channelPoolOptions.getCleanupInterval()); } return builder.build(); }

gcpChannelPoolOptions is always initialized to either merged user options. The builder setter also rejects null, adding a defensive null branch would just hide invariant breaks.

...nner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyAwareChannel.java

# Conflicts: # java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java

...nner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java

...nner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyAwareChannel.java

olavloite · 2026-04-09T10:12:04Z

...e-cloud-spanner/src/main/java/com/google/cloud/spanner/MultiplexedSessionDatabaseClient.java

    synchronized (CHANNEL_USAGE) {
-      CHANNEL_USAGE.putIfAbsent(sessionClient.getSpanner(), new BitSet(numChannels));
-      this.channelUsage = CHANNEL_USAGE.get(sessionClient.getSpanner());
+      SharedChannelUsage sharedChannelUsage = CHANNEL_USAGE.get(this.spanner);


I think that we should seriously consider removing this manual channel distribution logic from this class. It was introduced when using the Gax channel pool to prevent low-QPS from sticking to just one channel. My understanding is that grpc-gcp will do that automatically, as it falls back to a round-robin scheme when there is low load. That would significantly simplify this class without breaking the purpose of what this was intended to do.

Customers still have an option to disable grpc-gcp and switch to GAX so cleanup task should be done later.

olavloite · 2026-04-09T10:13:55Z

java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java

+    // read-only transactions tend to keep picking the same idle channel, so keep reads
+    // overlapping to verify distribution across the fixed-size pool.
+    mockSpanner.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofMinimumAndRandomTime(500, 0));


Would it be possible to just freeze the mock server and then wait for the server to contain N requests, and then unfreeze it, instead of adding 500ms execution time for each query? I think that would achieve the same, but with less execution speed. (There should be a util method in the mock server for 'waitForRequests' or something like that)

I tried this approach, but the current mock server freeze() is global and blocks earlier RPCs like session creation/transaction setup before enough ExecuteStreamingSql requests are enqueued. That made the test deadlock/time out. I kept the per-query streaming delay for now because it is deterministic and keeps the overlap in the specific RPC path we want to exercise.

If we want to switch to freeze/wait/unfreeze, I think we first need a mock-server utility that can either freeze only ExecuteStreamingSql or wait for an exact request count without globally blocking unrelated RPCs.

You can work around that by making sure the CreateSession RPC has finished before you start the actual test. There are at least two ways to achieve that:

Execute another query before freezing the mock server.

Use SessionPoolOptions#setWaitForMinSessions(..) to make sure that the creation of the session becomes a blocking operation.

I tried both workarounds:

prewarming with an extra query before freeze()

setting SessionPoolOptions#setWaitForMinSessionsDuration(...)

In this test path that still times out before the expected number of ExecuteStreamingSql requests reach the mock server, so the freeze/wait/unfreeze variant is still not stable here.

rahul2393 · 2026-04-09T11:13:07Z

java-spanner/.gitignore

@olavloite This was added in the commit https://github.com/googleapis/google-cloud-java/pull/12715/changes#diff-abde1c3268c5f4417d5742c6c8a5541c6cf3e7d05bfc0be134e044891f9dd181 causing github check failure

fix(spanner): fix grpc-gcp affinity cleanup and multiplexed channel u…

c94b6d4

…sage leaks

rahul2393 requested review from a team as code owners April 9, 2026 07:47

rahul2393 requested a review from olavloite April 9, 2026 07:49

gemini-code-assist bot reviewed Apr 9, 2026

View reviewed changes

Merge branch 'main' into fix-12693-b-494614610

ce64c9e

# Conflicts: # java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java

rahul2393 force-pushed the fix-12693-b-494614610 branch from 3599670 to ce64c9e Compare April 9, 2026 07:57

rahul2393 added 5 commits April 9, 2026 15:00

fix test

1b90d0d

Merge branch 'main' into fix-12693-b-494614610

c8db2c1

fix lint

2711774

refactored clearChannelHintAffinity

bb03696

fix graalVM test

0dc3d69

olavloite reviewed Apr 9, 2026

View reviewed changes

rahul2393 added 2 commits April 9, 2026 16:36

address comments

3288019

Merge branch 'main' into fix-12693-b-494614610

d7e900f

rahul2393 requested a review from olavloite April 9, 2026 11:08

fix github check related to .gitignore

5c4891d

rahul2393 commented Apr 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(spanner): fix grpc-gcp affinity cleanup and multiplexed channel usage leaks#12726

fix(spanner): fix grpc-gcp affinity cleanup and multiplexed channel usage leaks#12726
rahul2393 wants to merge 10 commits intomainfrom
fix-12693-b-494614610

rahul2393 commented Apr 9, 2026 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Apr 9, 2026

Uh oh!

rahul2393 Apr 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olavloite Apr 9, 2026

Uh oh!

rahul2393 Apr 9, 2026

Uh oh!

olavloite Apr 9, 2026

Uh oh!

rahul2393 Apr 9, 2026

Uh oh!

olavloite Apr 9, 2026

Uh oh!

rahul2393 Apr 9, 2026

Uh oh!

rahul2393 Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rahul2393 commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. Fix static CHANNEL_USAGE leak in MultiplexedSessionDatabaseClient

2. Stop using bitset / bounded % numChannels affinity for grpc-gcp

3. Add explicit grpc-gcp affinity cleanup for multi-use read-only transaction close

4. Apply grpc-gcp affinity cleanup settings without implicitly enabling DCP

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rahul2393 commented Apr 9, 2026 •

edited

Loading

1. Fix static `CHANNEL_USAGE` leak in `MultiplexedSessionDatabaseClient`

2. Stop using bitset / bounded `% numChannels` affinity for grpc-gcp